Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SWAP] Implement inference mode #2696

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lhs8928
Copy link
Contributor

@lhs8928 lhs8928 commented Aug 5, 2024

@taos-ci
Copy link

taos-ci commented Aug 5, 2024

📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2696. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/.

Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lhs8928, 💯 All CI checkers are successfully verified. Thanks.

@lhs8928 lhs8928 force-pushed the jihochu_picogpt_swap branch from 9163d60 to 1e1e782 Compare August 5, 2024 08:05
Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lhs8928, 💯 All CI checkers are successfully verified. Thanks.

@jijoongmoon
Copy link
Collaborator

@lhs8928
Copy link
Contributor Author

lhs8928 commented Aug 14, 2024

@DonghakPark Can you please check the clang-format version issue?

This patch is for inference mode for swap device.
It re-enable mmap feature, but writing time is controlled manually, due
to the inference mode handling.

Signed-off-by: Jiho Chu <[email protected]>
@lhs8928 lhs8928 force-pushed the jihochu_picogpt_swap branch from 1e1e782 to 42c1e8b Compare August 14, 2024 11:44
Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lhs8928, 💯 All CI checkers are successfully verified. Thanks.

Copy link
Member

@SeoHyungjun SeoHyungjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

/**
* @brief Constructor
*
* @param value value to set, defaults to current directory
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param value value to set, defaults to current directory
* @param value value to set, defaults to "train" mode

TensorLifespan var_ls = TensorLifespan::MAX_LIFESPAN;
TensorLifespan var_ls = swap_mode == "inference"
? TensorLifespan::FORWARD_INFER_LIFESPAN
: TensorLifespan::MAX_LIFESPAN;
TensorLifespan grad_ls = TensorLifespan::BACKWARD_FUNC_LIFESPAN;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pure question (it may be irrelevant to this PR, though).
Do we need grad_ls for the inference swap mode? Is it related to the loss thing?

Copy link
Contributor

@EunjuYang EunjuYang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for a minor suggestion on the comment.

@@ -88,7 +89,7 @@ void *SwapDevice::getBuffer(off_t offset, size_t size, bool alloc_only) {
<< "SwapDevice: seek file: " << dev_path;

len = read(fd, ptr, size);
NNTR_THROW_IF(len != (ssize_t)size, std::runtime_error)
NNTR_THROW_IF(len != (size_t)size, std::runtime_error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to cast 'ssize_t' to 'size_t'?
because it looks 'read func' returns 'ssize_t' type.

Copy link
Member

@DonghakPark DonghakPark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants